Skip to content

Conversation

@ineersa
Copy link
Contributor

@ineersa ineersa commented Oct 5, 2025

Added optional structuredContent for CallToolResult which enables return of structured content when we provide output schema for a tool.

Added ability to return CallToolResult from tool directly, which enables proper return of tool errors to allow LLM to self-correct tool call.

Motivation and Context

While testing my MCP server I've noticed inconsistencies between output with Python MCP server.

  1. There was no possibility and no implementation for structuredContent
  2. There was no possibility to return isError=true

Also while outputSchema being added in #88 , with outputSchema MCP server MUST provide structured content alongside text representation.

How Has This Been Tested?

Tested on my MCP server to ensure same response between PHP and Python versions.
Added unit tests.

Breaking Changes

No breaking changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@ineersa ineersa force-pushed the structured-content-return branch from f340e0d to cfbfff8 Compare October 5, 2025 01:26
@chr-hertel chr-hertel added the Server Issues & PRs related to the Server component label Oct 6, 2025
@ineersa ineersa force-pushed the structured-content-return branch from cfbfff8 to 1f4ccdf Compare October 8, 2025 01:49
]);

return new CallToolResult($formattedResult);
return CallToolResult::fromArray($formattedResult);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not fond of this change, I think it'd be better to have some kind of Result that has both contents and structured content:

  • we could still return a collection of content's (and add structured content if the result has some)
  • at some point we will probably only return structured content.

For example:

StructuredContentResult {
  public StructuredContent
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean something like

StructuredContentResult implements CallToolResult {
  public StructuredContent
  public array $contents
}

ContentResult implements CallToolResult {
  public array $contents
}

interface CallToolResult extends \JsonSerializable{
}

Then we can expect instance of CallToolResult interface as return and maybe skip

/** @var array<int, TextContent|ImageContent|EmbeddedResource|AudioContent|StructuredContent> $formattedResult */
$formattedResult = $toolReference->formatResult($result);

and expect user to return proper response from tool?

at some point we will probably only return structured content.

Structured content is not required while outputSchema is not set, and I can argue that for some tasks text response works better.
At least for now content response required even if structuredContent is set.

This implementation is far from best, so if you could steer me in right direction would be great.

I used it in my browser-mcp tools, most crucial part was ability to return proper tool usage errors with hints to help model correct tool calls. Structured content provided no improvements for tool calling and quality so I just removed it from final version.

Copy link
Contributor

@soyuka soyuka Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using this:

<?php

namespace ApiPlatform\Mcp\Schema\Result;

use Mcp\Schema\JsonRpc\ResultInterface;

class StructuredContentResult implements ResultInterface
{
    /**
     * Create a new StructuredContentResult.
     *
     * @param array     $structuredContent The JSON content
     * @param ResultInterface $result A traditional result
     */
    public function __construct(
        public array $structuredContent,
        public readonly ?ResultInterface $result = null
    ) {
    }

    public function jsonSerialize(): mixed
    {
        return ['structuredContent' => $this->structuredContent] + ($this->result?->jsonSerialize() ?? []);
    }
}

This is merely a suggestion :).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed implementation, removed some changes.

Now we have CallToolResultInterface and 2 classes implementing it:

  • CallToolResult as our general result class
  • CallToolStructuredContentResult as SturcturedContent result which pretty much decorates CallToolResult

Tested on my Browser MCP it's working as expected.

Also added Unit tests to cover behavior.

As for example of usage, maybe we can add one after #88 merged as they go hand by hand together.

@chr-hertel @CodeWithKyrian could you take a look when will get time, it's ready for proper CR now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused - the PR description doesn't match the code or do I miss something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of enabling tools to return the CallToolResult optionally if they want to have more control, but let's stick with one implementation here.

If we patch it in the lib we don't need an additional decoration, but can extend the CallToolResult with an optional structure argument, that is omitted in case it's empty. That would be also more consistent with what we have in other data classes and makes it easier in change scenarios. currently spec changes are only additive for example and users relying on using those classes would need to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused - the PR description doesn't match the code or do I miss something?

Yeah, I changed implementation a lot after previous comment and did rework, I can update it.
Comments here describe main idea more.

If we patch it in the lib we don't need an additional decoration, but can extend the CallToolResult with an optional structure argument, that is omitted in case it's empty. That would be also more consistent with what we have in other data classes and makes it easier in change scenarios. currently spec changes are only additive for example and users relying on using those classes would need to change.

Okay, gonna rework to keep single CallToolResult with optional structuredContent.

I like the idea of enabling tools to return the CallToolResult optionally if they want to have more control, but let's stick with one implementation here.

That was actually even more beneficial for me to return proper isError to LLM in response with hints and error explanation so it could do correction, if isError=false always returned as it is now LLM don't always understand what happened and that it needs to correct tool call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chr-hertel Moved structuredContent to CallToolResult. Fixed description. Updated unit tests accordingly. Tested responses in inspector, works as expected.

@ineersa ineersa force-pushed the structured-content-return branch 2 times, most recently from 619034f to 538549f Compare October 23, 2025 01:28
@chr-hertel chr-hertel changed the title Structured content return and tool error [Server] Structured content return and tool error Oct 23, 2025
@ineersa ineersa force-pushed the structured-content-return branch from cdff778 to e20862f Compare October 23, 2025 21:17
@ineersa ineersa force-pushed the structured-content-return branch from 8e3f9c7 to 6da70b8 Compare October 23, 2025 21:32
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Step by step - thanks @ineersa! :)

@chr-hertel chr-hertel merged commit d347c84 into modelcontextprotocol:main Oct 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants